Improved glyph url handling: accept arbitrary suffixes after {end} #1443 #1444
Conversation
…plibre#1443 - Fixed rust formatting
…plibre#1443 - Fixed clippy error
…plibre#1443 - Fixed follow up clippy error
| } | ||
|
|
||
| #[route( | ||
| "/font/{fontstack}/{start}-{end}", | ||
| "/font/{fontstack}/{start}-{end}*", |
There was a problem hiding this comment.
the only reason you are adding the above code is to ignore possible trailing exstentsions, right?
Why not restrict this to a pattern like below (and adding it to FontRequest) which should™️ work as far as I read the docs here.
Relevant part:
Replacement markers can optionally specify a regular expression which will be used to decide whether a path segment should match the marker. To specify that a replacement marker should match only a specific set of characters as defined by a regular expression, you must use a slightly extended form of replacement marker syntax. Within braces, the replacement marker name must be followed by a colon, then directly thereafter, the regular expression. The default regular expression associated with a replacement marker
[^/]+matches one or more characters which are not a slash. For example, under the hood, the replacement marker {foo} can more verbosely be spelled as{foo:[^/]+}. You can change this to be an arbitrary regular expression to match an arbitrary sequence of characters, such as{foo:\d+}to match only digits.
I have not tested that it actually works.
| "/font/{fontstack}/{start}-{end}*", | |
| "/font/{fontstack}/{start}-{end}{extension:[^\d]*}", |
Could you add a testcase to ensure that this works and that there are no regressions?
Actix-webs' testing infratstructure is documented here
There was a problem hiding this comment.
This looks like a good suggestion. I will try this out. Thanks for looking at the change.
I have attempted to fix #1443 for myself just to challenge myself to write some Rust. I'm a total Rust newbie so please review with care.
This pull requests attempts to solve the problem that many styles out there add suffixes like '.pbf' to the glyph url at the end (see #1443 for examples). The change adds some parsing code to FontRequest that allows arbitrary suffixes after {{end}} and only returns the the leading digits of {{end}} as number for get_font_range. Everything after that is discarded.
I hope that this way, the font server "just works" for most people and don't hit the issue that I did.